(new server) add websocket handling #524
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds WebSocket handling functionality to the new server implementation. The changes include adding a new serializeV2 function for message serialization and implementing comprehensive WebSocket support for real-time communication between clients and the server.
- Adds WebSocket endpoint handling for various message types (spaces, invitations, updates, etc.)
- Implements real-time connection management and broadcasting capabilities
- Extends existing services to support WebSocket operations
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/hypergraph/src/messages/serialize.ts | Adds new serializeV2 function that returns parsed object instead of string |
| packages/create-hypergraph/package.json | Version bump to 0.5.4 |
| packages/create-hypergraph/CHANGELOG.md | Adds changelog entry for version 0.5.4 |
| apps/server-new/src/websocket.ts | New comprehensive WebSocket handler supporting multiple message types |
| apps/server-new/src/services/updates.ts | New service for handling update operations with retry logic |
| apps/server-new/src/services/spaces.ts | Extended with WebSocket-related methods for space management |
| apps/server-new/src/services/space-inbox.ts | Adds method for retrieving latest space inbox messages |
| apps/server-new/src/services/invitations.ts | New service for managing invitations |
| apps/server-new/src/services/connections.ts | New service for managing WebSocket connections and broadcasting |
| apps/server-new/src/services/account-inbox.ts | Extended with methods for account inbox management |
| apps/server-new/src/server.ts | Integrates WebSocket layer and removes placeholder implementation |
| apps/server-new/src/index.ts | Minor logging configuration change |
| apps/server-new/src/http/api.ts | Changes status endpoint from / to /status |
| .claude/settings.local.json | Adds Claude permissions configuration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| return JSON.parse( | ||
| JSON.stringify(obj, (_key, value) => { | ||
| if (value instanceof Uint8Array) { | ||
| return { __type: 'Uint8Array', data: Array.from(value) }; | ||
| } | ||
| if (value instanceof Date) { | ||
| return { __type: 'Date', data: value.toISOString() }; | ||
| } | ||
| return value; | ||
| }), | ||
| ); |
There was a problem hiding this comment.
The serializeV2 function performs unnecessary JSON.stringify followed by JSON.parse. This creates a performance overhead compared to a direct object transformation. Consider implementing a recursive object transformation function instead of the stringify-parse roundtrip.
| return JSON.parse( | |
| JSON.stringify(obj, (_key, value) => { | |
| if (value instanceof Uint8Array) { | |
| return { __type: 'Uint8Array', data: Array.from(value) }; | |
| } | |
| if (value instanceof Date) { | |
| return { __type: 'Date', data: value.toISOString() }; | |
| } | |
| return value; | |
| }), | |
| ); | |
| function transform(value: any): any { | |
| if (value instanceof Uint8Array) { | |
| return { __type: 'Uint8Array', data: Array.from(value) }; | |
| } | |
| if (value instanceof Date) { | |
| return { __type: 'Date', data: value.toISOString() }; | |
| } | |
| if (Array.isArray(value)) { | |
| return value.map(transform); | |
| } | |
| if (value && typeof value === 'object') { | |
| const result: any = {}; | |
| for (const key in value) { | |
| if (Object.prototype.hasOwnProperty.call(value, key)) { | |
| result[key] = transform(value[key]); | |
| } | |
| } | |
| return result; | |
| } | |
| return value; | |
| } | |
| return transform(obj); |
Copilot uses AI. Check for mistakes.
| return { | ||
| id: key.id, | ||
| nonce: key.keyBoxes[0].nonce, | ||
| ciphertext: key.keyBoxes[0].ciphertext, | ||
| accountAddress, | ||
| authorPublicKey: key.keyBoxes[0].authorPublicKey, | ||
| }; |
There was a problem hiding this comment.
Potential array index out of bounds error. The code accesses key.keyBoxes[0] without checking if the keyBoxes array has any elements. This could cause a runtime error if a key has no keyBoxes.
| return { | |
| id: key.id, | |
| nonce: key.keyBoxes[0].nonce, | |
| ciphertext: key.keyBoxes[0].ciphertext, | |
| accountAddress, | |
| authorPublicKey: key.keyBoxes[0].authorPublicKey, | |
| }; | |
| if (!key.keyBoxes || key.keyBoxes.length === 0) { | |
| return []; | |
| } | |
| const keyBox = key.keyBoxes[0]; | |
| return [{ | |
| id: key.id, | |
| nonce: keyBox.nonce, | |
| ciphertext: keyBox.ciphertext, | |
| accountAddress, | |
| authorPublicKey: keyBox.authorPublicKey, | |
| }]; |
Copilot uses AI. Check for mistakes.
| spaceId: invitation.spaceId, | ||
| }; | ||
| } | ||
| console.error('Invalid space state from the DB', result.left); |
There was a problem hiding this comment.
Using console.error directly violates logging best practices in Effect applications. Use Effect.logError instead to maintain consistency with the logging framework.
| console.error('Invalid space state from the DB', result.left); | |
| Effect.logError('Invalid space state from the DB', result.left).runSync(); |
Copilot uses AI. Check for mistakes.
1 participant
No description provided.